Skip to content

fix(lark): link feedback cards to persisted messages#447

Open
glow-Kev1n wants to merge 2 commits into
scitix:mainfrom
glow-Kev1n:fix/siclaw-channel-feedback-message-location
Open

fix(lark): link feedback cards to persisted messages#447
glow-Kev1n wants to merge 2 commits into
scitix:mainfrom
glow-Kev1n:fix/siclaw-channel-feedback-message-location

Conversation

@glow-Kev1n

Copy link
Copy Markdown
Contributor

Summary

  • return the persisted final assistant message ID from the Lark response collector
  • embed that ID in new Feishu feedback card actions and forward it through chat.recordFeedback
  • keep legacy cards without message_id compatible
  • omit feedback buttons when the final assistant message could not be persisted, while still delivering the answer

Root cause

Feishu feedback cards previously carried only the session and CardKit card ID. That was sufficient to identify the card, but not to unambiguously correlate an older card with its persisted assistant turn.

message_ref remains the card aggregation and re-vote idempotency key. The new message_id is used only for the exact chat-message link.

The field is optional on callbacks and RPC requests so cards created by older Runtime versions remain compatible.

Validation

  • npx vitest run src/gateway/channels/lark-card.test.ts src/gateway/channels/lark.test.ts — 144 tests passed
  • npm run build — passed

Both checks ran on the development host with Node.js 22.22.3.

@glow-Kev1n
glow-Kev1n marked this pull request as ready for review July 23, 2026 15:31

@LikiosSedo LikiosSedo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One P2 behavior regression found in the Runtime side; the exact-message linkage and the paired Sicore handling otherwise look sound.

const isAnswer = !agentError && resultText.trim().length > 0;
const ok = await finalizeCard(larkClient, cardSession, finalCardBody,
isAnswer ? { ctx: { sessionId, channelId }, locale } : undefined);
isAnswer && assistantMessageId

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve feedback for delta-only replies

collectChannelResponse explicitly supports successful streams that end without an assistant message_end by falling back to parts.join("") (and lark.test.ts already covers that shape). In that path resultText is non-empty but assistantMessageId stays null, so this new guard removes the feedback row even though the previous code added one. That makes feedback disappear for a supported channel response shape. Please either persist the synthesized final text to obtain its exact message ID, or intentionally fall back to a legacy card without message_id, and add an end-to-end assertion for the delta-only card flow.

@LikiosSedo LikiosSedo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 7975b2c. The delta-only response path now persists the synthesized assistant reply, links the feedback card to the exact message ID, and has focused collector plus end-to-end Lark coverage. CI is green; no blocking issues remain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants